Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SPSite: Fix: SPSite Constructur returns $null when run as Ressource on SPSE #1443

Merged
merged 6 commits into from
Jan 23, 2025
Merged

SPSite: Fix: SPSite Constructur returns $null when run as Ressource on SPSE #1443

merged 6 commits into from
Jan 23, 2025

Conversation

ChristophHannappel
Copy link
Contributor

@ChristophHannappel ChristophHannappel commented Nov 27, 2024 •
edited by ykuijs
Loading

Pull Request (PR) description

The Get Method failed to get an existing Site Collection on SharePoint Server Subscription Edition

This Pull Request (PR) fixes the following issues

Task list

  • Added an entry to the change log under the Unreleased section of the file CHANGELOG.md.
    Entry should say what was changed and how that affects users (if applicable), and
    reference the issue being resolved (if applicable).
  • Resource documentation added/updated in README.md.
  • Resource parameter descriptions added/updated in README.md, schema.mof and comment-based
    help.
  • Comment-based help added/updated.
  • Localization strings added/updated in all localization files as appropriate.
  • Examples appropriately added/updated.
  • Unit tests added/updated. See DSC Community Testing Guidelines.
  • Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
  • New/changed code adheres to DSC Community Style Guidelines.

This change is Reviewable

@ykuijs
Copy link
Member

ykuijs commented Dec 27, 2024

The unit tests are failing for SPSE. Can you please fix those, so this PR can be merged and I can release a new version with both your fixes?

@ChristophHannappel
Copy link
Contributor Author

Hello, sorry for the late reply. I'll work on this next week. Thanks you

ChristophHannappel and others added 4 commits January 21, 2025 12:12
@ChristophHannappel
Merge branch 'master' into fix/SPSite
9b0923a
@ChristophHannappel
Fix: Unittest "The site doesn't exist yet and should" for SPSSE
cbaf833
@ChristophHannappel
Adds SPSE Support for VSCode Launch Options
a088b7d
@ChristophHannappel
Fix: Integration Test
b2e161e 
The site doesn't exist yet and should: Mock for Get-SPSite on SPSE returns the correct values

The site exists, but doesn't have default groups configured: Add test parameter "CreateDefaultsGroups" so the Test-TargetResource does test for default groups
@ChristophHannappel
Copy link
Contributor Author

Hi @ykuijs

i've added a Parameter CreateDefaultGroups to the The site exists, but doesn't have default groups configured Test

SharePointDsc/tests/Unit/SharePointDsc/SharePointDsc.SPSite.Tests.ps1

Lines 349 to 354 in e039c98

Context -Name "The site exists, but doesn't have default groups configured" -Fixture {
BeforeAll {
$testParams = @{
Url = "http://site.sharepoint.com"
OwnerAlias = "DEMO\User"
}

to honor the intention of the Test-TargetRessource Function:

SharePointDsc/SharePointDsc/DSCResources/MSFT_SPSite/MSFT_SPSite.psm1

Lines 542 to 545 in e039c98

if ($PSBoundParameters.ContainsKey("CreateDefaultGroups") -eq $true -and `
$CreateDefaultGroups -eq $true)
{
if ($CurrentValues.CreateDefaultGroups -ne $true)

The function only tests for the default groups if the parameter is used. Previously the test was successfull because the URL Property between the $testParams and the $CurrentValues is different. Which might be a missing Mock of New-Object inside this Context.

Since the Parameter CreateDefaultGroups has a default value it might be a good idea to change the Test Function from

SharePointDsc/SharePointDsc/DSCResources/MSFT_SPSite/MSFT_SPSite.psm1

Lines 542 to 545 in e039c98

if ($PSBoundParameters.ContainsKey("CreateDefaultGroups") -eq $true -and `
$CreateDefaultGroups -eq $true)
{
if ($CurrentValues.CreateDefaultGroups -ne $true)

to

 if ($CreateDefaultGroups -eq $true) 
 {
    if ($CurrentValues.CreateDefaultGroups -ne $true)

@codecov Codecov
Copy link

codecov bot commented Jan 22, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84%. Comparing base (e039c98) to head (b2e161e).
Report is 7 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1443   +/-   ##
======================================
  Coverage      84%     84%           
======================================
  Files         145     145           
  Lines       22809   22812    +3     
======================================
+ Hits        19244   19253    +9     
+ Misses       3565    3559    -6
Files with missing lines Coverage Δ
...PointDsc/DSCResources/MSFT_SPSite/MSFT_SPSite.psm1 84% <100%> (+2%) ⬆️

@ChristophHannappel ChristophHannappel marked this pull request as ready for review January 23, 2025 08:25
ykuijs
ykuijs approved these changes Jan 23, 2025
View reviewed changes
Copy link
Member

@ykuijs ykuijs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ykuijs ykuijs merged commit db1d423 into dsccommunity:master Jan 23, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ykuijs ykuijs ykuijs approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SPSite: Microsoft.SharePoint.SPSite Constructur returns $null when run as Ressource
2 participants
@ChristophHannappel @ykuijs